Move tap from core into Viz extension #5651
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Pothulapati
reviewed
Feb 3, 2021
viz/tap/pkg/util.go
Outdated
| @@ -0,0 +1,184 @@ | |||
| package pkg | |||
There was a problem hiding this comment.
Should this be util instead? referencing this as viz/tap/pkg was really confusing as pkg is this higher level packages directory in golang
There was a problem hiding this comment.
I feel like pkg is okay. I see util naming kind of equivalent; they are both packages that have general helpers that are imported by other packages.
There was a problem hiding this comment.
I split these into some better named files but still within tap/pkg package. Hopefully that helps in some way.
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
adleong
reviewed
Feb 3, 2021
…split Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
adleong
reviewed
Feb 4, 2021
There was a problem hiding this comment.
Tested and everything works as expected for me. However, I do see a slightly concerning warning in the tap and tap-injector logs:
2021/02/04 00:20:17 WARNING: proto: file "tap.proto" is already registered
previously from: "github.com/linkerd/linkerd2/viz/tap/gen/tap"
currently from: "github.com/linkerd/linkerd2-proxy-api/go/tap"
A future release will panic on registration conflicts. See:
https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict
Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
|
@adleong Thanks good catch. Renamed to |
adleong
approved these changes
Feb 4, 2021
…split Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com>
jijeesh
pushed a commit
to jijeesh/linkerd2
that referenced
this pull request
Mar 23, 2021
Closes linkerd#5545. This change moves all tap and tap-injector code into the viz directory. The tap and tap-injector components now also use a new tap image—separating these components from the controller image that they are currently part of. This means the controller image has removed all its build dependencies related to tap. Finally, the tap Protobuf has been separated from the metrics-api and moved into it's own `.proto` file and gen directory. This introduces a clear split between metrics-api and tap Protobuf. There is no change in behavior for the `viz tap` command. ### Reviewing #### Docker images All the bin directory scripts should be updated to build and load the tap image. All the CI workflows should be updated to build and push the tap image. #### Controller and pkg directories This is primarily deletions. Most of the deleted code in this directory is now in the tap directory of the Viz extension. #### viz/tap This is the location that all the tap related code now lives in. New files are mostly moved from the controller and pkg directories. Imports have all been updated to point at the right locations and Protobuf. The Protobuf here is taken from metrics-api and contains all tap-related Protobuf. Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com> Signed-off-by: Jijeesh <jijeesh.ka@gmail.com>
jijeesh
pushed a commit
to jijeesh/linkerd2
that referenced
this pull request
Apr 21, 2021
Closes linkerd#5545. This change moves all tap and tap-injector code into the viz directory. The tap and tap-injector components now also use a new tap image—separating these components from the controller image that they are currently part of. This means the controller image has removed all its build dependencies related to tap. Finally, the tap Protobuf has been separated from the metrics-api and moved into it's own `.proto` file and gen directory. This introduces a clear split between metrics-api and tap Protobuf. There is no change in behavior for the `viz tap` command. ### Reviewing #### Docker images All the bin directory scripts should be updated to build and load the tap image. All the CI workflows should be updated to build and push the tap image. #### Controller and pkg directories This is primarily deletions. Most of the deleted code in this directory is now in the tap directory of the Viz extension. #### viz/tap This is the location that all the tap related code now lives in. New files are mostly moved from the controller and pkg directories. Imports have all been updated to point at the right locations and Protobuf. The Protobuf here is taken from metrics-api and contains all tap-related Protobuf. Signed-off-by: Kevin Leimkuhler <kevin@kleimkuhler.com> Signed-off-by: Jijeesh <jijeesh.ka@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5545.
This change moves all tap and tap-injector code into the viz directory.
The tap and tap-injector components now also use a new tap image—separating
these components from the controller image that they are currently part of. This
means the controller image has removed all its build dependencies related to
tap.
Finally, the tap Protobuf has been separated from the metrics-api and moved into
it's own
.protofile and gen directory. This introduces a clear split betweenmetrics-api and tap Protobuf.
There is no change in behavior for the
viz tapcommand.Reviewing
Docker images
All the bin directory scripts should be updated to build and load the tap image.
All the CI workflows should be updated to build and push the tap image.
Controller and pkg directories
This is primarily deletions. Most of the deleted code in this directory is now
in the tap directory of the Viz extension.
viz/tap
This is the location that all the tap related code now lives in. New files are
mostly moved from the controller and pkg directories. Imports have all been
updated to point at the right locations and Protobuf.
The Protobuf here is taken from metrics-api and contains all tap-related
Protobuf.
Signed-off-by: Kevin Leimkuhler kevin@kleimkuhler.com